fix: use button tag and native tabindex for clear button #63
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #63 +/- ##
=======================================
Coverage 92.34% 92.34%
=======================================
Files 5 5
Lines 222 222
Branches 79 77 -2
=======================================
Hits 205 205
Misses 17 17 ☔ View full report in Codecov by Sentry. |
Walkthrough此次更改涉及到三个主要文件，分别是 Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
assets/index.less (1)
25-28: 清除按钮样式修改符合无障碍设计要求样式更改合理地移除了按钮的默认外观，同时保持了视觉一致性。这些更改与将清除图标从
<span>改为<button>的改动相匹配，有效地提升了组件的可访问性。建议考虑添加以下样式来进一步优化按钮的交互体验：
.rc-input-clear-icon { padding: 0; font-size: 12px; background: none; border: none; + cursor: pointer; + &:focus { + outline: 1px solid #000; + outline-offset: 2px; + } }src/BaseInput.tsx (1)
Line range hint
83-97: 使用原生button标签改善了可访问性，代码变更合理！将清除图标从span改为button标签是一个很好的改进：
- 提供了原生键盘交互支持
- 不再需要显式设置tabIndex和role属性
- 符合语义化HTML最佳实践
建议考虑添加以下无障碍增强：
- 为按钮添加aria-label，以提供更好的屏幕阅读器支持
- 添加type="button"属性，防止在表单中意外提交
<button + type="button" + aria-label="清除输入内容" onClick={(event) => { handleReset?.(event); onClear?.(); }}tests/BaseInput.test.tsx (3)
80-98: 建议使用 userEvent 简化点击测试当前测试使用了多个独立的鼠标事件（mouseDown、click、mouseUp），建议使用 userEvent 来简化测试代码，使其更接近真实用户交互。
建议修改如下：
- fireEvent.mouseDown(clearIcon!); - fireEvent.click(clearIcon!); - fireEvent.mouseUp(clearIcon!); + await user.click(clearIcon!);
100-126: 建议优化键盘交互测试以减少重复代码Space 和 Enter 键的测试逻辑非常相似，建议使用 test.each 模式来减少代码重复。
建议重构如下：
- it('By focus and Space', async () => { - const { container } = render(<Demo />); - - const inputEl = container.querySelector('input'); - await user.click(inputEl!); - - await user.type(inputEl!, 'some text'); - expect(inputEl!.value).toBe('some text'); - - await user.tab(); - await user.keyboard('[Space]'); - expect(inputEl!.value).toBe(''); - }); - - it('By focus and Enter', async () => { - const { container } = render(<Demo />); - - const inputEl = container.querySelector('input'); - await user.click(inputEl!); - - await user.type(inputEl!, 'some text'); - expect(inputEl!.value).toBe('some text'); - - await user.tab(); - await user.keyboard('[Enter]'); - expect(inputEl!.value).toBe(''); - }); + it.each([ + ['Space', '[Space]'], + ['Enter', '[Enter]'] + ])('By focus and %s', async (name, key) => { + const { container } = render(<Demo />); + + const inputEl = container.querySelector('input'); + await user.click(inputEl!); + + await user.type(inputEl!, 'some text'); + expect(inputEl!.value).toBe('some text'); + + await user.tab(); + await user.keyboard(key); + expect(inputEl!.value).toBe(''); + });
Line range hint
49-126: 建议添加焦点管理测试当前测试用例没有验证清除操作后输入框的焦点状态。为了确保良好的用户体验，建议添加相关测试。
建议添加如下测试：
it('应保持输入框焦点在清除后', async () => { const { container } = render(<Demo />); const inputEl = container.querySelector('input'); await user.type(inputEl!, 'some text'); await user.tab(); await user.keyboard('[Enter]'); expect(document.activeElement).toBe(inputEl); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
tests/__snapshots__/index.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
assets/index.less(1 hunks)src/BaseInput.tsx(2 hunks)tests/BaseInput.test.tsx(3 hunks)
🔇 Additional comments (4)
src/BaseInput.tsx (2)
Line range hint 84-91: 事件处理逻辑设计合理！
点击事件和鼠标按下事件的处理方式值得肯定：
- 通过preventDefault()防止失焦，解决了#31200问题
- 同时调用内部handleReset和外部onClear保持了良好的扩展性
Line range hint 91-96: 样式类名处理得当！
类名逻辑保持了组件的一致性：
- 正确处理了清除按钮的显示/隐藏状态
- 与后缀图标的样式兼容性良好
- 保持了原有的样式系统
tests/BaseInput.test.tsx (2)
2-2: 引入 userEvent 是个很好的改进！
使用 @testing-library/user-event 可以更真实地模拟用户交互，这是一个值得肯定的改进。
49-55: 测试结构优化得当！
将 allowClear 相关测试组织在一个 describe 块中，并共享测试设置，这样的重构提高了代码的可维护性和可读性。
Summary by CodeRabbit
新特性
<span>元素更改为<button>元素，提高了可访问性和交互性。样式
.rc-input-clear-icon的样式，移除了填充、背景和边框。测试
BaseInput组件的测试，增加了多个清除输入的交互场景。